Skip to content

Update on.xml #670

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Update on.xml #670

wants to merge 2 commits into from

Conversation

AurelioDeRosa
Copy link
Member

Fixed issue #665

@AurelioDeRosa
Copy link
Member Author

@arthurvr Any thought on this PR?

@dmethvin
Copy link
Member

dmethvin commented Mar 1, 2015

It will not be executed the following times because it's correctly removed.

This was confusing to me, maybe it was "the following times" since it isn't completely clear what that means. I think it could just be removed.

This behavior goes against the W3C events specification. It's worth noting that this is an edge case and this behavior allows to keep the logic simple.

I would just remove this. It's such an edge case that spending too much time explaining it is likely to confuse rather than enlighten people. The example helps for those who care.

@AurelioDeRosa
Copy link
Member Author

This was confusing to me, maybe it was "the following times" since it isn't completely clear what that means. I think it could just be removed.

Are you against the note or the wording? I feel like this is an important information to have.

It's worth noting that this is an edge case and this behavior allows to keep the logic simple.

Let's remove this part only?

@dmethvin
Copy link
Member

dmethvin commented Mar 1, 2015

It's definitely a balance between too many words and not enough. How about changing this:

This means that even if you try to remove an event handler inside another one for the same event executed first, the removed handler will be executed anyway.

to this?

Adding or removing event handlers on the current element won't take effect until the next time the event is handled. To prevent any further event handlers from executing on an element within an event handler, call event.stopImmediatePropagation().

Or something similar.

The thing that's strange about the W3C behavior is that it says removeEventListener takes effect immediately but addEventListener has no similar guarantee even though it could (it would have to be added on the end of the list). That asymmetry, plus the fact that nobody has complained in 9 years, tells me that this is pretty rare.

@AurelioDeRosa
Copy link
Member Author

I think your suggestion is fine. I'm going to update the PR so that we can merge it.

PR updated based on the discussion
@arthurvr arthurvr closed this in f161bb0 Mar 6, 2015
@AurelioDeRosa AurelioDeRosa deleted the patch-2 branch March 6, 2015 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants